Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[RNMobile] Fix text formatting mode lost after backspace is used #37676

Merged
merged 7 commits into from
Jan 5, 2022
Merged

[RNMobile] Fix text formatting mode lost after backspace is used #37676

merged 7 commits into from
Jan 5, 2022

Conversation

fluiddot
Copy link
Contributor

@fluiddot fluiddot commented Dec 31, 2021 •
edited
Loading

Fixes #34938.

Related to:

Description

This PR fixes the issue #34938 by introducing a couple of changes in Aztec-Android and Aztec-iOS. The main purpose of the change is to prevent Aztec modifying internally the active text format when the text changes and letting the RichText component have full control over it.

NOTE: Further information about the issue and this approach can be found in this comment #34938 (comment).

How has this been tested?

iOS:

  1. In a paragraph block, turn on bold.
  2. Type some text, notice that the text will be bold as expected.
  3. Delete the last character(s), notice that the bold format button is in its ON state.
  4. Again type text, notice that the new text is bolded.

Android:

  1. In a paragraph block, turn on bold.
  2. Type some text, notice that the text will be bold as expected.
  3. Move the text caret to the end of the word, notice that the bold format button is in its OFF state.
  4. Again type text, notice that the new text is NOT bolded, which is expected.
  5. Delete characters until the caret is placed after the last bolded letter, notice that the bold format button is in its OFF state.
  6. Again type text, notice that the new text is NOT bolded, which is expected.

Screenshots

iOS

Before After
Kapture.2021-12-31.at.13.34.42.mp4
ios-format-backspace-fix.mp4

Android

Before After
android-format-backspace-issue.mp4
Screen_Recording_20211231-165915_Gutenberg.mp4

Types of changes

Bug fix

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • I've tested my changes with keyboard and screen readers.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR (please manually search all *.native.js files for terms that need renaming or removal).

@fluiddot fluiddot added [Type] Bug An existing feature does not function as intended Mobile App - i.e. Android or iOS Native mobile impl of the block editor. (Note: used in scripts, ping mobile folks to change) labels Dec 31, 2021
@fluiddot fluiddot self-assigned this Dec 31, 2021
@fluiddot fluiddot mentioned this pull request Dec 31, 2021
Closed
fluiddot
fluiddot commented Dec 31, 2021
View reviewed changes
packages/react-native-editor/ios/Podfile Outdated
Comment on lines 8 to 11
# Changes Aztec reference to last commit from https://github.com/wordpress-mobile/AztecEditor-iOS/pull/1346 PR.
# ONLY FOR TESTING, IT SHOULD BE DELETED ONCE THE AZTEC PR IS MERGED!
pod 'WordPress-Aztec-iOS', :git => 'https://github.com/wordpress-mobile/AztecEditor-iOS.git', :commit => 'd68c76ec8f01490c741077dccefc17b17ce19f2d'

Copy link
Contributor Author

@fluiddot fluiddot Dec 31, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I updated the Aztec-iOS reference so it points to the PR (wordpress-mobile/AztecEditor-iOS#134) where the shouldRecalculateTypingAttributesOnDeleteBackward variable was added. Once that PR is merged, we should remove the changes made to Podfile and Podfile.lock files.

This was referenced Jan 3, 2022
Merged
Merged
@fluiddot fluiddot marked this pull request as ready for review January 3, 2022 09:27
@fluiddot fluiddot requested review from guarani and antonis January 3, 2022 09:27
@fluiddot fluiddot changed the title [RNMobile] Fix text formatting mode lost after backspace [RNMobile] Fix text formatting mode lost after backspace is used Jan 3, 2022
@fluiddot fluiddot mentioned this pull request Jan 3, 2022
Merged
2 tasks
antonis
antonis reviewed Jan 4, 2022
View reviewed changes
Copy link
Member

@antonis antonis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work @fluiddot 👍
I tested the Android implementation with a local build and it worked as expected 🎉
The code changes on the iOS side both on this PR and on Aztec LGTM but I haven't tested yet. I will have a look tomorrow if @guarani does not beat me to this 😄

guarani
guarani approved these changes Jan 4, 2022
View reviewed changes
Copy link
Contributor

@guarani guarani left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested on iOS and works as expected! ❤️ The code level changes on iOS also look good 👍

@fluiddot
Merge branch 'trunk' into rnmobile/fix/text-format-lost-after-backspace
8af474c 
# Conflicts:
#	packages/react-native-editor/CHANGELOG.md
#	packages/react-native-editor/ios/Podfile
#	packages/react-native-editor/ios/Podfile.lock
@fluiddot
Update Aztec version for Android
37e61c1
@fluiddot
Update WordPress-Aztec-iOS version to 1.19.7
daa9ac4
@fluiddot fluiddot merged commit 343ca61 into trunk Jan 5, 2022
@fluiddot fluiddot deleted the rnmobile/fix/text-format-lost-after-backspace branch January 5, 2022 13:46
@github-actions github-actions bot added this to the Gutenberg 12.4 milestone Jan 5, 2022
This was referenced Feb 2, 2022
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@antonis antonis antonis left review comments

@guarani guarani guarani approved these changes

Assignees

@fluiddot fluiddot

Labels
Mobile App - i.e. Android or iOS Native mobile impl of the block editor. (Note: used in scripts, ping mobile folks to change) [Type] Bug An existing feature does not function as intended
Projects
None yet
Milestone
Gutenberg 12.4
Development

Successfully merging this pull request may close these issues.

Text formatting mode lost after backspace is used
3 participants
@fluiddot @antonis @guarani